Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This commit will fix defect 28687 #59

Merged
merged 1 commit into from
Sep 20, 2015
Merged

This commit will fix defect 28687 #59

merged 1 commit into from
Sep 20, 2015

Conversation

sekler
Copy link
Contributor

@sekler sekler commented Sep 17, 2015

The defect described flows that are executed in delay, this was caused by a false triggering of the defense mechanism in the InBuffer that does not pull new messages when there are less than 50Mb of free memory, however this mechanism does not take into account GC, and the amount of memory that can be released as a result of GC

@oritstone
Copy link
Contributor

@sekler please write a comprehensive description of the bug you want to fix

@oritstone oritstone added the bug label Sep 17, 2015
@oritstone oritstone added this to the 0.9 - sprint 2 milestone Sep 17, 2015
@sekler
Copy link
Contributor Author

sekler commented Sep 17, 2015

Updated the commit description

if (System.currentTimeMillis() > (gcTimer + MINIMUM_GC_DELTA)){
logger.warn("Trying to initiate garbage collection");
//Todo find a better solution than manually triggering GC
System.gc();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a log msg after the GC ended? at least in debug...

@meirwah
Copy link

meirwah commented Sep 17, 2015

any test for this?

natabeck added a commit that referenced this pull request Sep 20, 2015
This commit will fix defect 28687
@natabeck natabeck merged commit 26cb178 into CloudSlang:master Sep 20, 2015
@orius123
Copy link
Contributor

Sorry for responding so late. But it just seem so wrong!! Why not wait for the JVM to trigger GC? And really, there should really be a flag to turn this hack off.

@orius123
Copy link
Contributor

And last but completely not least. Don't add todos to code, either open an issue or rethink-it. score code is not for handling tasks.

@natabeck
Copy link
Contributor

Hi :) That is the issue. That in current check if we see that the memory is about to end we do not poll - meaning do not ask java for more memory. That is why JVM does not do GC. It stays this way for minutes not polling and not doing GC (proven in profiler). We could completely remove the check of memory, but then we will fail on OutOfMemory or we can ask JVM to GC and once it is done we can continue to poll. Regarding the flag to disable it - I agree. Will be added.

@sekler
Copy link
Contributor Author

sekler commented Sep 24, 2015

This fixed was issued because we saw flows being delayed for minutes due to false positive check on the free memory, the bottom line is that our current check is naive, and does not take into account what can be freed in the heap, IMO it's either the added GC, or we should remove this check completely as it does not reflect the actual state of the JVM

@meirwah
Copy link

meirwah commented Sep 24, 2015

Agree with @orius123 on the flag to turn this off!!

@meirwah
Copy link

meirwah commented Sep 24, 2015

#60 : Add a flag to turn GC hack off

@orius123
Copy link
Contributor

And I agree with @natabeck and @sekler and say that the flag should remove the entire memory check, because it is not a complete check w/o the GC stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants